Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make bloat check work with github actions #1441

Merged
merged 57 commits into from
Jul 9, 2020

Conversation

andy31415
Copy link
Contributor

Problem

We moved from CircleCI to github (which supports IPv6). Need to update bloat check since APIs are probably different.

Summary of Changes

Exploring changes needed to get bloat checks to work again

@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@project-chip project-chip deleted a comment from lgtm-com bot Jul 3, 2020
@andy31415
Copy link
Contributor Author

Another idea would be for PRs to upload their binaries as usual and have the bloat check run in the scheduled process (we delete the outputs when bloats are generated). This would have the benefit of easily figuring out if a bloat report already ran or not.

@gerickson
Copy link
Contributor

Another idea would be for PRs to upload their binaries as usual and have the bloat check run in the scheduled process (we delete the outputs when bloats are generated). This would have the benefit of easily figuring out if a bloat report already ran or not.

I think @jwhui and the OpenThread team has some of these problems addressed. Are there solutions we can leverage there?

@woody-apple
Copy link
Contributor

@jelderton @robszewczyk @hawk248 ?

@andy31415
Copy link
Contributor Author

Another idea would be for PRs to upload their binaries as usual and have the bloat check run in the scheduled process (we delete the outputs when bloats are generated). This would have the benefit of easily figuring out if a bloat report already ran or not.

I think @jwhui and the OpenThread team has some of these problems addressed. Are there solutions we can leverage there?

I believe the solution there was a github app - I had the link when we previously discussed this.
@jwhui - did you have any issues with fork repo permissions during the openthread bloat checker setup?

@andy31415
Copy link
Contributor Author

https://github.com/apps/size-report/ was mentioned in #541

@andy31415
Copy link
Contributor Author

andy31415 commented Jul 6, 2020

https://github.com/apps/size-report/ was mentioned in #541

Docs are a bit light there. I believe I currently know what to do to make our bloaty-based reports work, however I cannot estimate what it takes to get size-report to work. I would prefer if someone else more familiar tries for the size-report integration and then we can compare (or if one approach seems a time sink, we should just keep with whatever we get working first)

@andy31415
Copy link
Contributor Author

@bukepo or @jwhui - do you want to try the path of making size-report work now that we are not a private repo anymore?

@bukepo
Copy link
Contributor

bukepo commented Jul 7, 2020

@andy31415 could you install the size-report app on this project, so that I can try bringing it up on this project?

@andy31415
Copy link
Contributor Author

@andy31415 could you install the size-report app on this project, so that I can try bringing it up on this project?

I don't think I have sufficient rights. @woody-apple: can we install the size-report app?

@andy31415
Copy link
Contributor Author

Separately, for this PR in particular: @saurabhst @jelderton @robszewczyk ?

@woody-apple
Copy link
Contributor

@andy31415 could you install the size-report app on this project, so that I can try bringing it up on this project?

I don't think I have sufficient rights. @woody-apple: can we install the size-report app?

Done!

@CLAassistant
Copy link

CLAassistant commented Jul 7, 2020

CLA assistant check
All committers have signed the CLA.

@woody-apple
Copy link
Contributor

@andy31415 can you sign the CLA? we can merge then :)

@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2020

This pull request fixes 2 alerts when merging dbc5cb7 into efa630b - view on LGTM.com

fixed alerts:

  • 1 for Unused named argument in formatting call
  • 1 for Unused import

@andy31415 andy31415 force-pushed the 01_bloat_in_github branch from dbc5cb7 to 4351126 Compare July 9, 2020 14:29
@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2020

This pull request fixes 2 alerts when merging 4351126 into ace290d - view on LGTM.com

fixed alerts:

  • 1 for Unused named argument in formatting call
  • 1 for Unused import

@andy31415
Copy link
Contributor Author

This will apparently require some iterations to fix - still unclear to me why builds fail since this was supposed to be a python only change.

@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2020

This pull request fixes 2 alerts when merging 59daba0 into ace290d - view on LGTM.com

fixed alerts:

  • 1 for Unused named argument in formatting call
  • 1 for Unused import

@andy31415 andy31415 merged commit 67a1b59 into project-chip:master Jul 9, 2020
@andy31415 andy31415 deleted the 01_bloat_in_github branch October 9, 2020 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants